-
Notifications
You must be signed in to change notification settings - Fork 2
Backend v2 #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Backend v2 #4
Conversation
Fix: getLogs limits, request chunking, and daily rewards
feat: add price range endpoint
Fix/limits v2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a backend v2 architecture that completely removes legacy components and introduces a new web3 client system for smart contract interaction. The changes include removing the old timer and testnet modules, and adding a comprehensive event scanning and smart contract integration framework.
- Complete removal of legacy timer and testnet modules
- Introduction of a new web3 client with contract factory and event scanning capabilities
- Implementation of WebSocket-based event monitoring for smart contracts
Reviewed Changes
Copilot reviewed 92 out of 109 changed files in this pull request and generated 8 comments.
File | Description |
---|---|
timer.py | Completely removed legacy uwsgi timer functionality |
testnet.py | Removed testnet configuration module |
src/web3client/* | New comprehensive web3 client implementation with event scanning, contract interfaces, and WebSocket support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
event_queue = EventQueueManager(w3=w3, log=log, start_block=start_block, max_run_depth=config.ws_max_run_depth, get_logs_cap=config.get_logs_cap) | ||
|
||
if len(config.vesting_contract_details) > 0 : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the extra space before the colon. Python style guide recommends no space before colons in conditionals.
if len(config.vesting_contract_details) > 0 : | |
if len(config.vesting_contract_details) > 0: |
Copilot uses AI. Check for mistakes.
for event in events: | ||
existing_topic = self.topic_map.get(event().topic) | ||
if existing_topic is None: | ||
print(f"Adding topic f{event.name}: {event().topic}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use logging instead of print statements. There appears to be a logging object available as self.log that should be used for consistency with the rest of the codebase.
print(f"Adding topic f{event.name}: {event().topic}") | |
self.log.debug(f"Adding topic {event.name}: {event().topic}") |
Copilot uses AI. Check for mistakes.
get_logs_calls = 0 | ||
for e in queue: | ||
from_block = e.start_block if e.start_block else start_block | ||
e.get_logs_calls=(ceil((block_current - from_block) / self.get_logs_cap) if self.get_logs_cap > 0 else 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add spaces around the assignment operator for better readability: e.get_logs_calls = (
e.get_logs_calls=(ceil((block_current - from_block) / self.get_logs_cap) if self.get_logs_cap > 0 else 1) | |
e.get_logs_calls = (ceil((block_current - from_block) / self.get_logs_cap) if self.get_logs_cap > 0 else 1) |
Copilot uses AI. Check for mistakes.
|
||
abis[name] = self.w3.eth.contract(abi=compiled_sol[key]["abi"]) | ||
return abis[name] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the contract key mapping logic into a separate method to reduce complexity in the get() method and improve maintainability.
key = self._get_contract_key(name) | |
abis[name] = self.w3.eth.contract(abi=compiled_sol[key]["abi"]) | |
return abis[name] | |
def _get_contract_key(self, name: str) -> str: | |
if name == "Ownable2StepUpgradeable": | |
return "node_modules/@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol:Ownable2StepUpgradeable" | |
elif name == "PausableUpgradeable": | |
return "node_modules/@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol:PausableUpgradeable" | |
elif name == "IERC1967": | |
return "node_modules/@openzeppelin/contracts/interfaces/IERC1967.sol:IERC1967" | |
elif name == "TokenVestingStaking": | |
return "utils/TokenVestingStaking.sol:TokenVestingStaking" | |
elif name == "TokenVestingNoStaking": | |
return "utils/TokenVestingNoStaking.sol:TokenVestingNoStaking" | |
else: | |
return f"{name}.sol:{name}" |
Copilot uses AI. Check for mistakes.
async def load_vesting_staking_contracts(w3: AsyncWeb3, details: list[VestingContractDetails]): | ||
contract_interface = TokenVestingStaking(w3=w3, db_writer=global_db_writer, log=log, event_queue=event_queue) | ||
#token_contract = Token(w3=w3, db_writer=global_db_writer, log=log).factory(details[0].SESH) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented-out code. If this code is needed for future reference, it should be documented in comments or tracked in version control history.
Copilot uses AI. Check for mistakes.
# assert SESH == known_details.SESH, f"Expected {known_details.SESH}, got {SESH}" | ||
# assert rewards_contract == known_details.rewards_contract, f"Expected {known_details.rewards_contract}, got {rewards_contract}" | ||
# assert sn_contrib_factory == known_details.sn_contrib_factory, f"Expected {known_details.sn_contrib_factory}, got {sn_contrib_factory}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the large block of commented-out code (lines 128-159). This makes the code harder to read and maintain. If this logic is needed for future implementation, document it properly or track it separately.
res = [] | |
contracts = [] | |
for known_details in details: |
Copilot uses AI. Check for mistakes.
) | ||
|
||
batch_items = 7 | ||
# TODO: use this to get details for old contracts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comment is too vague. Provide more specific information about what needs to be implemented or when this method should be used.
# TODO: use this to get details for old contracts | |
# TODO: Implement logic to retrieve and process contract details for legacy (pre-upgrade) service node contracts. | |
# This method should be used during migration or auditing tasks to collect parameters, operator info, | |
# BLS pubkeys, contributions, status, manual finalize state, and reserved contributors from older contracts. |
Copilot uses AI. Check for mistakes.
# assert self.transferable_beneficiary is not None | ||
# assert is_checksum_address(self.revoker) | ||
# assert is_checksum_address(self.SESH) | ||
# assert is_checksum_address(self.rewards_contract) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace vague TODO comments with specific actionable items or remove them if the decisions have been made. These comments don't provide useful guidance for future development.
# assert is_checksum_address(self.rewards_contract) |
Copilot uses AI. Check for mistakes.
https://gist.github.com/Aerilym/1a541dbb923498b04c89233293e02936
TODO: docs